fix(mobile): respect word wrap in diffs - #12590

Approvability

Verdict: Not approved

Macroscope's review found this PR not approvable — The PR adds a substantial cross-platform mobile diff-rendering capability with new layout engines, caching, asynchronous preparation, and changed scrolling and highlighting behavior. It also introduces new static-analysis suppressions, so the production impact and suppressions require human review.

^{You can add or adjust custom eligibility rules. Learn more.}

No actionable comments were generated in the recent review. 🎉

sequenceDiagram
  participant Appearance
  participant NativeStyle
  participant ReviewDiffView
  participant CodeLayout
  participant DiffRenderer
  Appearance->>NativeStyle: provide codeWordBreak
  NativeStyle->>ReviewDiffView: provide wordWrap
  ReviewDiffView->>CodeLayout: prepare wrapped row layouts
  CodeLayout->>ReviewDiffView: return row heights and visual lines
  ReviewDiffView->>DiffRenderer: draw wrapped code and diff highlights

• Fix all pre-merge checks with AI

_{Comment @coderabbitai help to get the list of available commands.}

Verified word-level diff highlighting on the installed native clients at ab3d9b3fbf9: iPhone 17 Pro / iOS 26.5 and Pixel 9 / Android 16.

Both deletion and addition highlights follow the changed text across wrap boundaries. Checked printable ASCII, CJK, emoji, combining accents, Arabic/BiDi, and tab-containing rows. Unchanged prefixes/suffixes remain outside the highlight. No code changes were needed.

The fixture uses one contiguous edit per row; the production range calculator confirms one UTF-16 range on each side for all six cases. Existing filtering intentionally suppresses word highlighting for more than four ranges or over 45% changed non-whitespace content, independently of native rendering.

Case	iOS	Android
ASCII and CJK
Emoji and combining accents
Arabic and tabs

Performance measurements for f95e22ae670 (original PR) → ab3d9b3fbf9 (fixed), with word highlights enabled.

UIKit native renderer — repeated before/after benchmark

Apple M4 Max, macOS 27.0, Mac Catalyst UIKit, optimized Swift. Seven measured fresh-view trials after one warm-up per revision/size. 80% ASCII / 10% CJK / 10% emoji + combining marks; viewport 390×844 points at 2×. Paint measurements invoke the production drawing method into a fresh bitmap, including text and word backgrounds; not a cached layer snapshot.

¹ Median of seven trial-level p95s, each from 31 paints. Other repeated operations use trial medians.

The tradeoff is real: correct Unicode measurement increases cold wall time to about 712 ms for 10k rows. Initial preparation runs on the decode worker; the median longest observed main-run-loop iteration falls from 13.46 to 2.31 ms. Cached comment toggles improve about 6×. Warm highlighted viewport paint is about 25% lower in this fixture; first paint is slower. Corrected wrapping changes visible geometry, so these are viewport costs, not equal-glyph throughput comparisons. Width/font invalidation was not benchmarked. Run-loop observations are not phone frame times, and bitmap paints are not GPU/presentation timings.

Android app scrolling — current revision only

Full debug app, Pixel 9 / Android 16 x86_64 emulator on Ryzen 7 8845HS, 60 Hz. T3 Device stream active, no screen recording. Used the real highlighted diff fixture (ASCII, CJK, emoji, accents, Arabic, tabs) over the isolated remote backend. Reset Android gfxinfo counters before each sample, then performed ten alternating 1300 px swipes with 700 ms pauses. Startup/navigation excluded.

Combined: 98/1468 frames (6.68%) missed the 16.7 ms deadline. This is not jank-free. These are whole-app Android gfxinfo measurements on a streamed debug emulator; they do not isolate the renderer or prove a regression/improvement. No original-PR Android frame baseline or physical-device/release measurement was taken. Sample windows include idle time, so dividing frame counts by wall time would not produce meaningful FPS.

Download raw samples, benchmark source, reproduction script, and methodology. No app-code changes for this measurement.

Profiled scrolling a complete 10,000-changed-row diff on Android at ab3d9b3fbf9. There is a measurable size-dependent bottleneck in the sticky header lookup, and deep scrolling exposes a syntax-highlighting bug.

Pixel 9 / Android 16 x86_64 debug emulator, 60 Hz, Ryzen 7 8845HS, Device stream active. One TypeScript file with 5,000 deleted + 5,000 added lines, 80% ASCII / 10% CJK / 10% emoji + combining accents, wrapping and word-change backgrounds enabled. Native logs confirm 10,044 entries including surrounding headers/other fixture entries. The 937 KB patch fits under the server's 1 MiB per-file preview limit.

Frame health, original source restored, no instrumentation or system trace running

Each pass: 20 one-way swipes, 1300 px, 700 ms pauses. Counters reset before the outbound pass; each frame read resets the next window. Loading and build activity excluded.

Combined 220/1,842 = 11.9%. Large-diff scrolling is not consistently smooth on this setup. This is a streamed debug emulator, not a physical-device release measurement.

Where the time goes

Trace slices overlap; do not add those elapsed times together. JavaScript consumed only 235 ms on CPU in the retained trace, versus 4,030 ms on the UI thread and 12,853 ms on RenderThread. It was not the dominant CPU consumer in this run.

Controlled code experiment

A temporary Android-only prototype replaced the linear header scans with an index. Native timers, first 120 draws discarded:

That removes about 36% of native draw time in this fixture. One run each, 720 vs 840 warm draws. Both use the same timers; only the baseline also had Perfetto active. Treat this as evidence for removing the header scan, not a controlled claim about end-to-end frame-rate improvement. The prototype was restored, and the original app rebuilt/reinstalled. No production changes committed for this profile.

Syntax highlighting stops following the viewport

Only the initial 239 rows received syntax tokens; at line ~1,674, syntax colors are absent while word-change backgrounds still work. The shared hook compares each event with the immediately previous event and then overwrites that reference. Movement of 1–2 rows per frame never accumulates to its 20-row threshold. Compare against the last requested/highlighted range instead, and guarantee settled-viewport coverage. This needs fixing before using these numbers to represent fully syntax-highlighted scrolling.

Once token delivery is corrected, also remeasure token-triggered full-row layout passes. They were not recurring during this scroll because highlighting had stopped; initial layout calls reached 26.8 ms in the instrumented run.

Initial rows	After scrolling to ~1,674

Download frame samples, native timers, trace summaries, fixture generator, and prototype/instrumentation scripts. Full Android Perfetto trace retained locally at /tmp/pr12590-scroll/10k-instrumented.perfetto-trace. Its bounded buffer covers the later part of the scroll. Simpleperf was denied by the emulator; the iOS Time Profiler attempt exited without a usable capture, so there is no iOS CPU-profile claim here.

Note: GPT-6 on behalf of shivam (@shivamhwp).

At f95e22ae, wrapping treats UTF-16 units as equal-width columns. Wider text such as CJK gets clipped, and horizontal scrolling is disabled, making that content inaccessible. Wrap boundaries and highlight positions need matching native measurements, with the ASCII shortcut kept where valid. Unchanged wrap results should also be reused when comments expand or collapse, rather than remeasuring every code line.

The Android comparison shows why startup performance also matters: the measured prototype fixed the clipping cases but increased layout time from about 13 to 39 ms for 1,000 mixed rows. Similar cached drawing times do not remove the initial layout cost.

I see ab3d9b3f now adds native text layout, caching, and regression coverage. The findings above apply to the earlier commit; we have not retested this update. Before merging, the remaining verification should cover CJK, combining marks, emoji, tabs, syntax styling, large-diff startup, and comment toggles on Android and iOS, including confirming that comment toggles do not remeasure unchanged code.

Fixed the syntax-highlighting starvation found in the large-diff profile in c3a864a.

The shared mobile hook now compares viewport movement against the last requested range. Small scroll events accumulate instead of resetting the threshold on every event. A 150 ms trailing refresh covers the final viewport when movement stops below the threshold; duplicate draw events do not postpone it. Pending refreshes are canceled when the diff changes or unmounts.

Validation:

• 19 focused scheduler/highlighter tests pass. The gradual-scroll regression goes through row 1,674 one row per event. Temporarily restoring the old per-event comparison makes that test fail with zero requests during scrolling.
• Mobile typecheck and targeted formatting pass. Targeted lint has no errors; the hook retains effect-reset warnings. React Doctor reports 100/100.
• iOS 18.6 simulator: opened the existing 10,000-changed-row TypeScript fixture, scrolled to the end, then reversed. Syntax colors and word-change backgrounds are visible at added lines 4,972–5,000 and again around 4,160. This change is shared by iOS and Android, but this follow-up visual pass was iOS only. No new frame-rate claim; the earlier profile ran with highlighting starved.

Reverse-scrolling recording

The physical-phone previews previously installed at ab3d9b3 have not been redeployed with this follow-up yet.

Implemented and verified by GPT-6 via Codex.